docs: Adds a guide to writing code #751
Conversation
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
from
884c96a to
35632dd
Compare
|
Shouldn't be this part of CONTRIBUTING.md ? |
Should be linked there at least |
|
I would not mix docs for package managers and coding best practices 🤷 |
Ideally not. Contributing guidelines are already quite bloated in content, so stylistic things should go to something like "HACKING.md"
Yes, agreed. |
docs/coding_best_practices.md
Outdated
|
|
||
| ## FAQ | ||
|
|
||
| Q. Is this guide exhaustive? |
There was a problem hiding this comment.
This doesn't render well
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
3 times, most recently
from
37e2b22 to
7a4e3be
Compare
|
|
||
| ### Write easy to follow code | ||
|
|
||
| - Try avoiding extremely long lines in any direction. Horizontal lines longer |
There was a problem hiding this comment.
We have a linter rule that covers this. Is it necessary to add this point here as well?
There was a problem hiding this comment.
Horizontal -- yes, vertical -- not really. I'd have kept it for completeness.
| - Consider variable names which reflect intended use and potentially types of | ||
| objects they will be bound with. | ||
|
|
||
| - While doing the above remember, that vowel shortage is over, so in most |
There was a problem hiding this comment.
I'm not sure I completely get what you mean here. Can you add a quick example?
There was a problem hiding this comment.
lst_pckgs and other overly compacted names. This is not a problem in the codebase yet and arguably I am the one most prone to using them (only in very narrow scopes, but nevertheless).
There was a problem hiding this comment.
"Vowel shortage" made me chuckle; very much felt.
| * [Write clear tests which actually test functionality](#write-clear-tests-which-actually-test-functionality) | ||
| * [FAQ](#faq) | ||
|
|
||
| ## Background |
There was a problem hiding this comment.
Altough I think this section is very well written and is pertinent, I'm afraid that the large text might scare off some people from reading. I feel like the two first paragraphs have the most important messages (we enforce good standards for maintainability, and reviews are done in good faith).
In any case, I'm not strongly against keeping this section as is, so please do so if you prefer.
There was a problem hiding this comment.
I was taking a wider look at both CONTRIBUTING.md and coding_best_practices.md, and they might be a little too much information for a newcomer. Maybe we can add some quick reference summaries later on, so one can get up to speed quickly.
There was a problem hiding this comment.
This is intended to be more of a supplementary text and not a must-read. All I do here is reiterate basics. I'll reword the referring sentence in CONTRIBUTING.md to highlight this.
| and [PEP20](https://peps.python.org/pep-0020); | ||
|
|
||
|
|
||
| ### Write clear tests which actually test functionality |
There was a problem hiding this comment.
Should we drop the test guidelines from CONTRIBUTING.md?
There was a problem hiding this comment.
No. That guide is the main one, this is supplementary.
|
|
||
| ## Best practices | ||
|
|
||
| ### Document your decisions |
There was a problem hiding this comment.
Should we drop the comment guidelines from CONTRIBUTING.md?
There was a problem hiding this comment.
I think no. The idea is that CONTRIBUTING.md is enough for most if not all contributors, and this is a fall-back for cases when we need to explain why certain piece of code does not pass a code review.
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
2 times, most recently
from
e8c0973 to
a82a3e6
Compare
This commit adds a reference document with some best practices to help resolve disputes when writing code. Related to: containerbuildsystem#737 Signed-off-by: Alexey Ovchinnikov <[email protected]>
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
from
a82a3e6 to
ea474d7
Compare
|
I really like what's here so far (not least because I strongly agree with all of it 🤣). Have you considered adding a link to The Zen of Python? I don't know how well-known it is these days, but IMO it's an invaluable resource (outside of Python, even). |
Yes, I have provided a link to 19 of the 20 postulates. Edit: L201, GH keeps rendering the page with the highlight just off screen. |
LOL, I had forgotten that it started life as a PEP 😆 |
|
My suggestion would be this:
|
|
@slimreaper35 thank you for the suggestions! Please let me start with quoting the newly added document:
To reiterate, this is not something new, I would expect that most if not all future contributors are well familiar with these concepts. The document is related to #737 and to a discussion that caused a submission of the aforementioned issue. The reason for this document is essentially the same as for a Code of Conduct: to have the obvious spelled out for the rare case when it is not obvious.
The document is linked to from CONTRIBUTING.md, is supplementary, is hopefully clearly marked as one, and does not contain any unique knowledge. This is a condensation of best practices known to mankind for at least thirty years (likely more, I arbitrarily count from GoF publication year to have some common reference point).
There is no need to (see the points above, below, and also this fragment from the document itself: "It is ... intended ... to be used as a reference and a disambiguation tool for code reviewers").
This document deals with the ways of communicating intent and not with styles. In other words the document explains how to be comprehensible. While there is an infinite number of pieces of code that a) compile and b) solve some problem only a small subset of those are comprehensible and, by induction maintainable. No one can maintain a bit of code that they cannot comprehend. Maintaining a bit of code that is hard to comprehend is not economically feasible as well. Assembly is a fine language and it is absolutely possible to write just in it if one is ready to pay premium for bug fixes and feature requests. (For another graphic example see code snippets in the "Background" section). Issue 737 arose from an argument about maintainability, one of complaints during the argument was about a lack of document with basics of clear coding practices. This PR adds such document. Please note, that this is not an original research: a multitude of books, conference talks, and blogposts have been created by others which address the same problems and recommend the same approaches in a more verbose and argumented way. This knowledge is so common that I did not even feel the need to trace any of the statements to their sources and did not include any references. I believe it is abundantly clear that I am not the original author of any of these suggestions and merely act as a scribe. Since this is already a preexisting knowledge, the document simply provides a common ground for rare cases when it is not.
Linters are not context-aware. The bulk of the document is concerned with logical organization and reasoning about why certain patterns work and why others do not (this especially applies to the parts about lengths and alignments). |
This commit adds a reference document with some
best practices to help resolve disputes when
writing code.
Related to: #737
Maintainers will complete the following section
Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:
/ok-to-test(as is the standard for Pipelines as Code)